Skip to content

Fixed: Livesync+Debug on Android not working when there are changes in tns-core-modules #1985

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Aug 11, 2016
Merged

Fixed: Livesync+Debug on Android not working when there are changes in tns-core-modules #1985

merged 1 commit into from
Aug 11, 2016

Conversation

tzraikov
Copy link
Contributor

@tzraikov tzraikov commented Aug 8, 2016 •
edited
Loading

The issue:

  1. Create a new {N} project.
  2. Open it in Visual Studio Code. Run debug with "Sync on Android". This will start livesync and then debug.
  3. Make a change in tns-core-modules.
  4. Run "Sync on Android" again. It will fail. This happens because the prepareProject called by livesync service will clean the android build folder with all apks. It is required by the debug service that is called after livesync.

Proposal:

In that situation we change only the js file and therefore it is not necessary to clean the whole build folder. We could check whether the change is in platforms folder of particular module. It so, we should clean the project. If not, it is safe to skip the clean process.

@tzraikov tzraikov force-pushed the raikov/fix-livesync-debug-android branch from 15f0142 to e72cc4f Compare August 8, 2016 16:09
rosen-vladimirov
rosen-vladimirov reviewed Aug 9, 2016
View reviewed changes
lib/commands/debug.ts

let applicationId = deviceAppData.device.isEmulator ? projectData.projectName : deviceAppData.appIdentifier;
let applicationId = deviceAppData.appIdentifier;
if (deviceAppData.device.isEmulator && deviceAppData.platform.toLowerCase() === this.$devicePlatformsConstants.iOS.toLocaleLowerCase()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toLocaleLowerCase -> toLowerCase maybe

@tzraikov tzraikov changed the title Fixed: Livesync on Android is not working after changes in tns-core-modules Fixed: Livesync+Debug on Android not working when there are changes in tns-core-modules Aug 9, 2016
@tzraikov tzraikov force-pushed the raikov/fix-livesync-debug-android branch 4 times, most recently from 59092f0 to 27d6214 Compare August 9, 2016 13:18
@Plamen5kov
Copy link
Contributor

Plamen5kov commented Aug 9, 2016 •
edited
Loading

This is a functionality that needs to be tested (unit testing not integration).

@tzraikov tzraikov force-pushed the raikov/fix-livesync-debug-android branch 3 times, most recently from 4b7124d to 4d315a4 Compare August 9, 2016 14:33
@tzraikov tzraikov force-pushed the raikov/fix-livesync-debug-android branch from 4d315a4 to a44cf9f Compare August 9, 2016 14:33
@tzraikov
Copy link
Contributor Author

tzraikov commented Aug 9, 2016

@Plamen5kov
Yes, as a general suggestion.. tests should be added. However, I don't see a single test related with livesync or debug functionality in our code. Does anyone have an idea how to test those features?

@rosen-vladimirov
Copy link
Contributor

With current code gradle clean will be skipped in Debug + LiveSync scenario, no matter what's really changed. As I believe the code will evolve in the future and the current solution will be only part of a bigger effort (as far as I understand livesync will successfully detect what has changed in a user project and will rebuild it in case it's required) I give my 👍 . Yes, the current code resolves an issue and introduces new one (always skipping gradle clean in a specific scenario), but I believe that this has been considered and it's the easiest solution for a bug that is a show stopper for one of our internal teams.
👍 and I like the updated description of the issue.

@tzraikov tzraikov mentioned this pull request Aug 10, 2016
Merged
@tzraikov tzraikov merged commit 6f13402 into master Aug 11, 2016
@tzraikov tzraikov deleted the raikov/fix-livesync-debug-android branch August 11, 2016 13:06
tzraikov added a commit that referenced this pull request Aug 11, 2016
@tzraikov
Merge pull request #1985 from NativeScript/raikov/fix-livesync-debug-…
a391f80 
…android

Fixed: Livesync+Debug on Android not working when there are changes in tns-core-modules
@tzraikov tzraikov added this to the 2.2.1 milestone Aug 12, 2016
@vchimev vchimev self-assigned this Aug 12, 2016
@vchimev vchimev removed their assignment Jun 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
2.2.1
Development

Successfully merging this pull request may close these issues.
4 participants
@tzraikov @Plamen5kov @rosen-vladimirov @vchimev